Fix Memory64Lowering table.grow return value for -1 failure#8318
Fix Memory64Lowering table.grow return value for -1 failure#8318kripken merged 3 commits intoWebAssembly:mainfrom
Conversation
visitTableGrow used a plain i64.extend_i32_u on the table.grow return value, but table.grow returns -1 on failure. i64.extend_i32_u(0xFFFFFFFF) produces 0x00000000FFFFFFFF (4294967295), not i64 -1 (0xFFFFFFFFFFFFFFFF). Code checking result == -1 would fail to detect the failure. Apply the same if/else -1 check pattern already used by visitMemoryGrow.
| (func $table-grow-minus-one (result i64) | ||
| ;; table.grow returns -1 on failure. The lowering must check for -1 and | ||
| ;; return i64(-1) instead of i64.extend_i32_u(i32(-1)) which would be | ||
| ;; 4294967295. |
There was a problem hiding this comment.
This looks like it duplicates the existing test in the file below? Let's just use that one, but please move the comment from here to there.
- Add comment explaining why curr->type is set to i32 (the table.grow node is reused as a child of local.tee storing to an i32 local) - Remove duplicate test file memory64-lowering-table-grow.wast - Move the -1 failure explanation comment to the existing test_table_grow function in memory64-lowering.wast
|
Addressed both comments:
|
kripken
left a comment
kripken
left a comment
There was a problem hiding this comment.
Looks good, thanks. Let's just remove the comment - good point that it is identical to the code above, and it isn't commented there either, this is just a general pattern in this file. So let's not make this place inconsistent.
3 participants
Summary
visitTableGrowused a plaini64.extend_i32_uon thetable.growreturn value, buttable.growreturns -1 (i32) on failure.i64.extend_i32_u(0xFFFFFFFF)produces0x00000000FFFFFFFF(4294967295), not i64 -1 (0xFFFFFFFFFFFFFFFF). Code checkingresult == -1would fail to detect the failure.The analogous
visitMemoryGrowalready has the correct handling: it uses alocal.tee+if/elseto check for -1 and returni64.const -1explicitly.Fix: Apply the same if/else -1 check pattern from
visitMemoryGrowtovisitTableGrow.Split from #8311 per review feedback.
Test plan
memory64-lowering.wasttest expectationsmemory64-lowering-table-grow.wastverifying the -1 handling